fix: resolve path errors and hard-coded dependencies in semantic segmentation example#371
fix: resolve path errors and hard-coded dependencies in semantic segmentation example#371krrish175-byte wants to merge 1 commit intokubeedge:mainfrom
Conversation
kubeedge-bot
added
kind/bug
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: krrish175-byte The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubeedge-bot
added
the
size/L
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the robustness and usability of the semantic segmentation example by addressing critical pathing issues and removing environment-specific hard-coded dependencies. The changes ensure that the example can run correctly across different environments without manual path adjustments, making it more accessible and easier to maintain for other developers. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
…antic segmentation Signed-off-by: krrish175-byte <krrishbiswas175@gmail.com>
krrish175-byte
force-pushed
the
fix-issue-307
branch
from
27814fd to
6951d05
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request does a great job of cleaning up the semantic segmentation examples by removing developer-specific absolute paths and commented-out configurations. This significantly improves the portability and readability of the code. I've identified a few remaining areas where hardcoded paths and values could be parameterized for even better flexibility, particularly concerning cache files, model checkpoints, log files, and CUDA device selection. My review includes specific suggestions to address these points.
| print("load cache") | ||
| else: | ||
| sam = sam_model_registry["vit_h"](checkpoint="/home/hsj/ianvs/project/segment-anything/sam_vit_h_4b8939.pth").to('cuda:1') | ||
| sam = sam_model_registry["vit_h"](checkpoint="/ianvs/project/segment-anything/sam_vit_h_4b8939.pth").to('cuda:1') |
There was a problem hiding this comment.
The SAM checkpoint path ("/ianvs/project/segment-anything/sam_vit_h_4b8939.pth") and the CUDA device ('cuda:1') are hardcoded. This reduces portability and makes configuration difficult. It's better to pass these as arguments, for instance via self.args. Using a hardcoded device ID is particularly brittle and assumes a specific multi-GPU setup. A better approach would be:
sam_checkpoint = getattr(self.args, "sam_checkpoint", "/ianvs/project/segment-anything/sam_vit_h_4b8939.pth")
device = f"cuda:{self.args.gpu_ids}" if self.args.cuda else "cpu"
sam = sam_model_registry["vit_h"](checkpoint=sam_checkpoint).to(device)| print("load cache") | ||
| else: | ||
| sam = sam_model_registry["vit_h"](checkpoint="/home/hsj/ianvs/project/segment-anything/sam_vit_h_4b8939.pth").to('cuda:1') | ||
| sam = sam_model_registry["vit_h"](checkpoint="/ianvs/project/segment-anything/sam_vit_h_4b8939.pth").to('cuda:1') |
There was a problem hiding this comment.
The SAM checkpoint path and CUDA device are hardcoded here as well, similar to the sam_predict_ssa method. These should be parameterized for better flexibility and portability.
sam_checkpoint = getattr(self.args, "sam_checkpoint", "/ianvs/project/segment-anything/sam_vit_h_4b8939.pth")
device = f"cuda:{self.args.gpu_ids}" if self.args.cuda else "cpu"
sam = sam_model_registry["vit_h"](checkpoint=sam_checkpoint).to(device)| with open('/ianvs/project/cache.pickle', 'rb') as file: | ||
| cache = pickle.load(file) |
There was a problem hiding this comment.
The cache file path /ianvs/project/cache.pickle is hardcoded. This makes the code less flexible and harder to configure for different environments. It would be better to define this path as a constant or pass it as a parameter. Furthermore, the current implementation will raise a FileNotFoundError if the cache file doesn't exist (e.g., on a fresh run). It's better to handle this gracefully, for example with a try...except block:
cache_path = getattr(self.args, "cache_path", "/ianvs/project/cache.pickle")
try:
with open(cache_path, 'rb') as file:
cache = pickle.load(file)
except (FileNotFoundError, EOFError):
cache = {}| with open('/ianvs/project/cache.pickle', 'rb') as file: | ||
| cache = pickle.load(file) |
There was a problem hiding this comment.
The cache file path /ianvs/project/cache.pickle is hardcoded here as well. This makes the code less flexible. Consider making it a configurable parameter and handling potential FileNotFoundError or EOFError as suggested for the sam_predict_ssa method.
cache_path = getattr(self.args, "cache_path", "/ianvs/project/cache.pickle")
try:
with open(cache_path, 'rb') as file:
cache = pickle.load(file)
except (FileNotFoundError, EOFError):
cache = {}2 participants
What type of PR is this?
/kind bug
/kind cleanup
What this PR does / why we need it:
This PR resolves path errors and removes hard-coded developer dependencies in the
robot/lifelong_learning_bench/semantic-segmentationexamples.Following the directory restructuring in PR #84, many configuration files were moved to the
semantic-segmentationsubdirectory, breaking their internal relative paths. Additionally, several YAML and Python configurations contained developer-specific absolute paths (e.g.,/home/...), which prevented the example from running in other environments.This PR:
/data/datasets/) and relative project paths (/ianvs/project/).#workspace,#testenv,#mode) to keep the files clean and readable.Which issue(s) this PR fixes:
Fixes #76